Skip to content

Add basic buffer pruner#1829

Open
yerzhan7 wants to merge 4 commits into
awslabs:feature/memory-limitfrom
yerzhan7:feature/memory-limit-pruning-skeleton
Open

Add basic buffer pruner#1829
yerzhan7 wants to merge 4 commits into
awslabs:feature/memory-limitfrom
yerzhan7:feature/memory-limit-pruning-skeleton

Conversation

@yerzhan7
Copy link
Copy Markdown
Contributor

@yerzhan7 yerzhan7 commented May 12, 2026

Skeleton for the background buffer pruner that will reclaim memory from idle prefetch handles under memory pressure.

What changed:

  • New WakeSignal primitive (Mutex<bool> + Condvar): coalescing single-consumer wake with optional timeout.
  • New maintenance thread that runs both periodic pool trim and on-demand pruning rounds. Replaces the standalone schedule_trim thread — one thread now handles both responsibilities.
  • MemoryLimiter gains trigger_pruning() (notify) and pruning_signal(); on Drop it notifies the signal so the maintenance thread observes the dead Weak and exits.
  • run_pruning_round skeleton: pool trim → check queue → defer to natural release path → drop one idle prefetch handle. Queue inspection and handle drop are stubs returning Idle/false for now.

Why Mutex<bool> + Condvar

  • Mountpoint does not have dependency on tokio.
  • Coalesces many notifies into one wake (no thrashing under bursty pressure).
  • Polls every 1ms while pressure is sustained (no CPU burn — the thread sleeps between rounds).
  • Stops polling and parks the thread when pressure clears.
  • No lost wakeups: notifies before wait_timeout set the pending flag and are observed on the next call.
  • Periodic 60s trim runs on the same thread when idle — same code path, no extra thread.

Sleep behavior:

  • Outer 60s idle wait is interruptible via trigger_pruning() — pressure starts → immediate wake.
  • Inner 1ms tick between drain rounds is not interruptible — under sustained pressure the loop already polls every 1ms, so an extra wake has nothing useful to do.

Does this change impact existing behavior?

No

Does this change need a changelog entry? Does it require a version change?

No


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

@yerzhan7 yerzhan7 temporarily deployed to PR integration tests May 12, 2026 10:06 — with GitHub Actions Inactive
Signed-off-by: Yerzhan Mazhkenov <20302932+yerzhan7@users.noreply.github.com>
@yerzhan7 yerzhan7 force-pushed the feature/memory-limit-pruning-skeleton branch from 770fe63 to afc223a Compare May 14, 2026 12:42
@yerzhan7 yerzhan7 requested a deployment to PR integration tests May 14, 2026 12:42 — with GitHub Actions Waiting
@yerzhan7 yerzhan7 requested a deployment to PR integration tests May 15, 2026 09:35 — with GitHub Actions Waiting
…ignal

Signed-off-by: Yerzhan Mazhkenov <20302932+yerzhan7@users.noreply.github.com>
@yerzhan7 yerzhan7 force-pushed the feature/memory-limit-pruning-skeleton branch from 952374c to 44d483a Compare May 15, 2026 11:08
@yerzhan7 yerzhan7 requested a deployment to PR integration tests May 15, 2026 11:08 — with GitHub Actions Waiting
Signed-off-by: Yerzhan Mazhkenov <20302932+yerzhan7@users.noreply.github.com>
@yerzhan7 yerzhan7 requested a deployment to PR integration tests May 18, 2026 08:45 — with GitHub Actions Waiting
Conflicts resolved:
- limiter.rs: drop the `active_reads` DashMap (subsumed by upstream's
  unified `cursors: DashMap<CursorId, Weak<CursorState>>` where each
  CursorState owns its `active_read: Mutex<Option<ActiveRead>>`).
  Keep `pruning_signal` and the pruning hooks.
- limiter.rs: rewrite `has_active_reads()` to iterate `cursors` and
  upgrade each Weak, since active-read state now lives per-cursor.
- pool.rs: drop the obsolete `inner_stats`/`inner_limiter` test helpers
  and the `reserve`/`try_reserve`/`release_cursor`/`next_cursor_id`
  delegation methods replaced by upstream's `create_cursor` API.
  Keep the `inner()` accessor for the maintenance test.
- pool.rs: re-add `#[cfg_attr(feature = "shuttle", allow(dead_code))]`
  on `inner()` since the maintenance test that uses it is gated
  `not(feature = "shuttle")`.
- memory.rs: keep modules in alphabetical order (`maintenance` between
  `limiter` and `pages`).

Signed-off-by: Yerzhan Mazhkenov <20302932+yerzhan7@users.noreply.github.com>
@yerzhan7 yerzhan7 temporarily deployed to PR integration tests May 18, 2026 08:59 — with GitHub Actions Inactive
@yerzhan7 yerzhan7 marked this pull request as ready for review May 18, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant